Skip to content

feat(backend): complete six template gaps for production readiness#33

Merged
GRACENOBLE merged 4 commits into
mainfrom
24-feat-backend-completions
Jun 23, 2026
Merged

feat(backend): complete six template gaps for production readiness#33
GRACENOBLE merged 4 commits into
mainfrom
24-feat-backend-completions

Conversation

@GRACENOBLE

@GRACENOBLE GRACENOBLE commented Jun 23, 2026

Copy link
Copy Markdown
Owner

Summary

  • User profile CRUD — migration adds firebase_uid NOT NULL UNIQUE, email, photo_url, updated_at to users; backfills existing rows before applying NOT NULL; PATCH /api/v1/me upserts profile; DELETE /api/v1/me removes the DB record
  • Welcome email wiredNewHandleWelcomeEmail(sender) constructor actually calls Mailjet; WelcomeEmailPayload + UserCreatedEvent now carry a Name field so recipients are addressed by name (falls back to email)
  • Redis Streams infrastructure completeStreamProducer interface wired through bootstrap → FCM handler publishes UserCreatedEvent on token registration; consumer intentionally not started at runtime (avoids holding a persistent idle Redis connection with no business logic); full wiring example left as a comment in cmd/api/main.go; Consumer client sets MaxIdleConns=1 + ConnMaxIdleTime=8s to prevent managed-Redis idle-kill noise when re-enabled
  • WebSocket bi-directionalReadPump parses inbound frames as Envelope; Hub.OnMessage(msgType, handler) routes them to registered handlers; 64-slot semaphore bounds concurrency; hub lifecycle context propagated to handlers
  • Pagination typesdomain.Page[T], domain.CursorPage[T], PageRequest with Defaults() / Offset() (guards against negative offset)
  • Shared validation helpersbindJSON / bindQuery in transport/handlers/validation.go; return stable messages ("invalid request body", "invalid query parameters") instead of raw err.Error()
  • Graceful shutdownStreamProducer.Close() called alongside Cache and Enqueuer; consumer goroutine (when wired) cancels via context before Redis client closes

Docs updated

routing.md, database.md, migrations.md, queue.md, streams.md, websocket.md, email.md, error-handling.md, _index.md — all updated to reflect new routes, types, patterns, and wiring status.

Test plan

  • cd backend && go build ./... — compiles clean
  • cd backend && go vet ./... — no issues
  • cd backend && go test ./internal/transport/handlers/... ./internal/usecase/... ./internal/infrastructure/ws/... ./internal/infrastructure/queue/... — unit tests pass
  • cd backend && go test -tags integration ./... — all 12 packages pass (requires Docker)
  • cd backend && make swagger — docs regenerate; PATCH /api/v1/me and DELETE /api/v1/me appear in Swagger UI

- Wire Redis Streams end-to-end: Producer initialised in bootstrap, consumer
  goroutine in server fans UserCreatedEvent → Asynq welcome email task
- Fix welcome email handler to actually call Mailjet via NewHandleWelcomeEmail
  constructor (was only logging before)
- Add user profile CRUD: migration adds firebase_uid/email/photo_url to users,
  PATCH /api/v1/me upserts profile, DELETE /api/v1/me removes DB record
- Make WebSocket bi-directional: ReadPump forwards inbound Envelopes to hub,
  Hub.OnMessage registers typed handlers dispatched per message type
- Add generic pagination types (Page[T], CursorPage[T], PageRequest) to domain
- Add bindJSON/bindQuery validation helpers; refactor existing handlers to use them
- Remove TestClose that closed shared testDB mid-suite, breaking later tests
@github-actions github-actions Bot added the area: backend Go REST API label Jun 23, 2026
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a domain.User model, Goose migration, and PostgreSQL user repository; introduces PATCH/DELETE /api/v1/me HTTP endpoints; wires a Redis Streams event pipeline from FCM token registration through a server consumer to the welcome-email queue; refactors the queue handler to accept an injected sender; and adds WebSocket inbound message dispatch with typed handler registration.

Changes

User profile management and event-driven welcome email pipeline

Layer / File(s) Summary
Domain model and usecase interfaces
backend/internal/domain/user.go, backend/internal/domain/pagination.go, backend/internal/usecase/user.go, backend/internal/usecase/streams.go
Defines domain.User struct, UserRepository and StreamProducer usecase interfaces, and generic Page/CursorPage/PageRequest pagination types.
DB migration and PostgreSQL user repository
backend/internal/infrastructure/database/migrations/..., backend/internal/infrastructure/database/postgres/user_repository.go, backend/internal/infrastructure/database/postgres/user_repository_test.go, backend/internal/infrastructure/database/postgres/health_repository_test.go
Goose migration adds profile columns and unique firebase_uid index to users; userRepository implements Upsert (INSERT … ON CONFLICT) and DeleteByFirebaseUID; end-to-end tests cover create, update, and delete; removes the now-unnecessary TestClose.
Welcome-email queue handler refactor
backend/internal/infrastructure/queue/handlers.go, backend/internal/infrastructure/queue/handlers_test.go, backend/cmd/api/main.go
Replaces the static HandleWelcomeEmail function with NewHandleWelcomeEmail(sender) constructor; nil sender skips sending; main.go switches registration to the new constructor.
Bootstrap StreamProducer wiring
backend/internal/bootstrap/bootstrap.go
Adds App.StreamProducer field and conditionally initializes it via streams.NewProducer(cfg.RedisURL) in Run, returning a wrapped error on failure.
Handler dependency injection and shared validation helpers
backend/internal/transport/handlers/handler.go, backend/internal/transport/handlers/validation.go, backend/internal/transport/handlers/swagger_types.go, backend/internal/transport/handlers/storage_handler.go, backend/internal/transport/handlers/health_handler_test.go
Adds streamProducer and userRepo fields to Handler; introduces bindJSON/bindQuery helpers; adds UserAlias swagger type alias; migrates PresignHandler to bindJSON; updates health tests for the extended constructor signature.
PATCH/DELETE /api/v1/me handlers, routes, and tests
backend/internal/transport/handlers/me_handler.go, backend/internal/transport/handlers/me_handler_test.go, backend/internal/transport/handlers/routes.go
Implements UpdateMeHandler and DeleteMeHandler with Firebase claims extraction and repository calls; routes are registered only when userRepo != nil; tests cover success, missing-body 400, and no-claims 401 cases.
FCM handler event publishing and server Streams consumer
backend/internal/transport/handlers/fcm_handler.go, backend/internal/server/server.go
FCM handler publishes UserCreatedEvent to StreamUserCreated after token registration; server.go instantiates userRepo, wires it into NewHandler, and runs a background consumer that reads the stream and enqueues welcome-email jobs.
Swagger spec updates
backend/docs/swagger/docs.go, backend/docs/swagger/swagger.json, backend/docs/swagger/swagger.yaml
Regenerated Swagger docs to document DELETE and PATCH /api/v1/me with BearerAuth, handlers.updateMeRequest request schema, and handlers.UserAlias response schema.

WebSocket inbound message dispatch

Layer / File(s) Summary
Inbound types, Hub expansion, and client ReadPump
backend/internal/infrastructure/ws/message.go, backend/internal/infrastructure/ws/hub.go, backend/internal/infrastructure/ws/client.go, backend/internal/infrastructure/ws/hub_test.go
Adds InboundMessage and InboundHandler types; expands Hub with inbound channel, msgHandlers map, and RWMutex; adds OnMessage registration; extends Run to dispatch inbound messages asynchronously; ReadPump now unmarshals and forwards messages into hub.inbound; new TestHub_OnMessage validates end-to-end handler invocation.

Sequence Diagram(s)

sequenceDiagram
  participant FCMHandler
  participant StreamProducer
  participant RedisStream
  participant ServerConsumer
  participant Enqueuer
  participant QueueWorker
  participant EmailSender

  FCMHandler->>StreamProducer: publish user created event
  StreamProducer->>RedisStream: append stream message
  ServerConsumer->>RedisStream: read user created message
  ServerConsumer->>Enqueuer: enqueue welcome email task
  Enqueuer->>QueueWorker: deliver task
  QueueWorker->>EmailSender: send welcome email
Loading
sequenceDiagram
  participant WSClient
  participant ReadPump
  participant Hub
  participant InboundHandler

  WSClient->>ReadPump: send websocket message
  ReadPump->>ReadPump: unmarshal envelope
  ReadPump->>Hub: push inbound message
  Hub->>Hub: lookup handler by message type
  Hub->>InboundHandler: invoke handler asynchronously
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

area: backend

🐇 A rabbit hops through streams and queues,
Upserts a user, spreads welcome news!
WebSocket pings now find their way,
/api/v1/me brightens the day.
Firebase claims, a DELETE, a PATCH—
Every profile finds its match! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 34.62% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'complete six template gaps for production readiness' directly matches the PR objectives, which list six implemented features (Redis Streams, email, user CRUD, WebSocket bi-directionality, pagination, and validation helpers).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 24-feat-backend-completions

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (1)
backend/internal/infrastructure/queue/handlers_test.go (1)

13-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the actual send path.

TestHandleWelcomeEmail_ValidPayload constructs the handler with a nil sender, so it only verifies the graceful no-op branch — the sender.SendWelcomeEmail(...) path and its argument mapping (p.Email, p.Email) are never exercised. Consider adding a test with a mock EmailSender that asserts it is invoked with the expected arguments.

As per coding guidelines for backend/**/*_test.go: "Use TDD approach: write failing tests first, then implement the functionality".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/internal/infrastructure/queue/handlers_test.go` around lines 13 - 23,
The current test TestHandleWelcomeEmail_ValidPayload only verifies behavior with
a nil sender, so the actual email sending path through
sender.SendWelcomeEmail(...) is never exercised. Add a second test that
constructs the handler with a mock or test double that implements EmailSender,
then verify that the mock's SendWelcomeEmail method is called exactly once with
the expected arguments (p.Email for both the email and display name parameters
from the WelcomeEmailPayload).

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/internal/bootstrap/bootstrap.go`:
- Around line 139-146: The StreamProducer created from streams.NewProducer() in
the bootstrap code owns a Redis client with a Close() method but is never closed
during graceful shutdown, causing a connection leak. The usecase.StreamProducer
interface only exposes the Publish method, preventing callers from closing the
resource. Add a Close() error method to the usecase.StreamProducer interface to
make the close operation part of the contract, then update the concrete
implementation in streams/producer.go to implement this method if not already
present, and finally invoke streamProducer.Close() in cmd/api/main.go during the
graceful shutdown sequence alongside the existing cleanup of Cache and Enqueuer
resources.

In `@backend/internal/domain/pagination.go`:
- Around line 19-36: The Offset() method can return a negative value when Page
is 0 because the binding constraint allows min=0, but pagination pages should
start at 1. Guard the Offset() method to ensure it handles unnormalized input by
applying the same default normalization logic that Defaults() uses, ensuring
Page is treated as at least 1 when calculating the offset value, so callers get
a valid result even if they forget to call Defaults() first.

In
`@backend/internal/infrastructure/database/migrations/20260623063851_add_user_profile.sql`:
- Around line 2-8: The firebase_uid column is currently nullable but serves as
the identity key, which violates the contract expected by the new API handler.
Add NOT NULL constraint to the firebase_uid column definition in the ALTER TABLE
users statement. Additionally, the separate CREATE UNIQUE INDEX statement on
firebase_uid is redundant since the column already has a UNIQUE constraint;
remove this index creation line. Before applying the NOT NULL constraint, ensure
any existing rows in the users table are backfilled with firebase_uid values to
prevent migration failures.

In `@backend/internal/infrastructure/queue/handlers.go`:
- Line 24: The SendWelcomeEmail method call in handlers.go is passing p.Email
for both the toEmail and toName parameters, which means the welcome email will
address the recipient by their email address instead of a proper display name.
To fix this, extend the WelcomeEmailPayload struct (from tasks.go) to include a
display name field, propagate this name through the upstream UserCreatedEvent,
and then use this display name when calling SendWelcomeEmail instead of passing
p.Email twice. If a display name is not available, at minimum document and
confirm that using the email address as a fallback for the recipient's name is
the intended behavior.

In `@backend/internal/infrastructure/ws/client.go`:
- Around line 65-72: The code currently silently ignores json.Unmarshal errors
when processing inbound WebSocket frames. When jsonErr is not nil in the
json.Unmarshal call on the Envelope type, no logging or error handling occurs
and the frame is dropped without any signal. Add an else block or separate error
handling path after the json.Unmarshal of the Envelope to log the error using
slog.Warn or slog.Error when unmarshaling fails, and continue processing
subsequent messages rather than silently swallowing the error.

In `@backend/internal/infrastructure/ws/hub.go`:
- Line 77: In the hub.go file where fn is being invoked with
context.Background() on line 77, replace context.Background() with the hub's
lifecycle context. This ensures that when the hub's Run method is cancelled, the
context passed to handler fn will also be cancelled, allowing handlers to
properly terminate instead of continuing to run. Use the context that is
associated with the hub's lifecycle (typically a context that is cancelled when
the hub stops running).
- Around line 75-80: The inbound handler in the hub spawns an unbounded
goroutine for each inbound message received, which can cause memory exhaustion
under bursty traffic. Replace the unbounded `go func(msg InboundMessage)`
goroutine launch with a bounded concurrency mechanism, such as a semaphore
channel or worker pool pattern, to limit the maximum number of concurrent
inbound handler executions. Ensure the error handling and logging behavior
within the fn(context.Background(), msg) call remains intact while enforcing the
concurrency limit.

In `@backend/internal/server/server.go`:
- Around line 65-84: The Redis Streams consumer goroutine has two issues that
need fixing. First, replace context.Background() in the consumer.Run call with a
cancellable context that is tied to the server lifecycle (similar to the
workerCancel pattern used in main.go), and ensure this context is cancelled
during graceful shutdown so that the Run method returns and allows the
consumer.Close() call to execute. Second, the error returned from
streams.NewConsumer at line 67 is silently discarded via the if err == nil guard
- instead log this error so that failed consumer setup is observable to
operators, following the guideline to never swallow errors.

In `@backend/internal/transport/handlers/fcm_handler.go`:
- Around line 54-59: The error returned by the streamProducer.Publish call in
the FCM handler is being silently discarded using the blank identifier. Instead
of ignoring the error with `_`, capture the error returned by
streamProducer.Publish when publishing the UserCreatedEvent and log it using an
appropriate logger at the error level. This ensures that any failures in the
publish operation are observable and can be debugged, while still allowing the
request to proceed as a best-effort operation.

In `@backend/internal/transport/handlers/validation.go`:
- Around line 11-13: The error handling in the ShouldBindJSON validation block
is exposing internal validation details by using err.Error() directly, which
violates the error-handling principle of never exposing internal error messages
to clients. Replace the err.Error() call with a stable, client-facing message
such as "invalid request body" in the JSON response to prevent leaking internal
details like field names and struct tags while still informing the client that
their request was malformed.

---

Nitpick comments:
In `@backend/internal/infrastructure/queue/handlers_test.go`:
- Around line 13-23: The current test TestHandleWelcomeEmail_ValidPayload only
verifies behavior with a nil sender, so the actual email sending path through
sender.SendWelcomeEmail(...) is never exercised. Add a second test that
constructs the handler with a mock or test double that implements EmailSender,
then verify that the mock's SendWelcomeEmail method is called exactly once with
the expected arguments (p.Email for both the email and display name parameters
from the WelcomeEmailPayload).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e7bb682e-0d8f-4fd5-8a48-af1e1d9b9043

📥 Commits

Reviewing files that changed from the base of the PR and between daae38b and 9800c09.

📒 Files selected for processing (29)
  • backend/cmd/api/main.go
  • backend/docs/swagger/docs.go
  • backend/docs/swagger/swagger.json
  • backend/docs/swagger/swagger.yaml
  • backend/internal/bootstrap/bootstrap.go
  • backend/internal/domain/pagination.go
  • backend/internal/domain/user.go
  • backend/internal/infrastructure/database/migrations/20260623063851_add_user_profile.sql
  • backend/internal/infrastructure/database/postgres/health_repository_test.go
  • backend/internal/infrastructure/database/postgres/user_repository.go
  • backend/internal/infrastructure/database/postgres/user_repository_test.go
  • backend/internal/infrastructure/queue/handlers.go
  • backend/internal/infrastructure/queue/handlers_test.go
  • backend/internal/infrastructure/ws/client.go
  • backend/internal/infrastructure/ws/hub.go
  • backend/internal/infrastructure/ws/hub_test.go
  • backend/internal/infrastructure/ws/message.go
  • backend/internal/server/server.go
  • backend/internal/transport/handlers/fcm_handler.go
  • backend/internal/transport/handlers/handler.go
  • backend/internal/transport/handlers/health_handler_test.go
  • backend/internal/transport/handlers/me_handler.go
  • backend/internal/transport/handlers/me_handler_test.go
  • backend/internal/transport/handlers/routes.go
  • backend/internal/transport/handlers/storage_handler.go
  • backend/internal/transport/handlers/swagger_types.go
  • backend/internal/transport/handlers/validation.go
  • backend/internal/usecase/streams.go
  • backend/internal/usecase/user.go
💤 Files with no reviewable changes (1)
  • backend/internal/infrastructure/database/postgres/health_repository_test.go

Comment thread backend/internal/bootstrap/bootstrap.go
Comment thread backend/internal/domain/pagination.go Outdated
Comment thread backend/internal/infrastructure/queue/handlers.go Outdated
Comment thread backend/internal/infrastructure/ws/client.go
Comment thread backend/internal/infrastructure/ws/hub.go Outdated
Comment thread backend/internal/infrastructure/ws/hub.go Outdated
Comment thread backend/internal/server/server.go Outdated
Comment thread backend/internal/transport/handlers/fcm_handler.go
Comment thread backend/internal/transport/handlers/validation.go
GRACENOBLE and others added 3 commits June 23, 2026 10:10
- Add Close() to StreamProducer interface; close producer on graceful shutdown
- Move streams consumer goroutine from server.go to main.go with a proper
  cancellable context (streamCancel) so it exits cleanly on SIGINT/SIGTERM
- Log consumer init failure instead of silently dropping via if err == nil guard
- Add Name field to UserCreatedEvent and WelcomeEmailPayload so welcome emails
  address recipients by name instead of their email address
- Guard Offset() against page <= 1 to prevent negative SQL OFFSET values
- Make firebase_uid NOT NULL in migration; backfill legacy rows before constraint
- Log publish errors in fcm_handler instead of swallowing with _ =
- Bound inbound WebSocket handler concurrency with a 64-slot semaphore
- Pass hub lifecycle context to inbound handlers instead of context.Background()
- Log malformed inbound WebSocket frames instead of silently discarding
- Use stable error messages in bindJSON/bindQuery to avoid leaking internals
- Use bindJSON helper in UnregisterFCMToken (was still using inline pattern)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Managed Redis providers (Upstash, Redis Cloud) terminate idle connections
after ~10 s, which surfaces as an i/o timeout on the blocking XReadGroup
call. go-redis reconnects automatically on the next attempt, so these are
not real errors — log them at WARN and retry immediately.

For genuine unexpected errors, sleep 500 ms before retrying so the loop
does not hammer Redis when it is actually unhealthy.
…o use

The blocking XReadGroup poll holds a persistent Redis connection even when
there is no business logic consuming events, wasting a connection slot on
managed Redis plans. The consumer is not wired at startup until a concrete
use case exists.

All infrastructure remains in place (Producer, Consumer, events, types) and
the producer still publishes UserCreatedEvent on FCM registration. A fully
worked example of how to wire the consumer back in is left as a comment in
cmd/api/main.go.

Also adds MaxIdleConns=1 and ConnMaxIdleTime=8s to the consumer client so
idle connections are recycled before managed Redis providers kill them,
preventing i/o timeout noise if the consumer is re-enabled.
@GRACENOBLE GRACENOBLE merged commit 6c8a18a into main Jun 23, 2026
3 checks passed
@GRACENOBLE GRACENOBLE deleted the 24-feat-backend-completions branch June 24, 2026 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: backend Go REST API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant